-
Notifications
You must be signed in to change notification settings - Fork 54
Joins #763
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Database/Database.php (1)
1-1: Fix PSR-12 indentation failure flagged by Pint.The linter reports a PSR-12 statement indentation error in this file (Line 1). Please run Pint or adjust indentation to satisfy PSR-12.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/e2e/Adapter/Scopes/JoinsTests.php`:
- Around line 34-35: The PHPDoc `@var` annotations incorrectly name the variable
`$database` while the actual variable is `$db`; update each docblock to read
`@var Database $db` so PHPStan recognizes the correct variable type—apply this
change to the docblock immediately above the `$db = $this->getDatabase();`
assignment and the other occurrences that reference `$database` in the same test
class (replace `@var Database $database` with `@var Database $db`).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Database/Query.php (2)
1271-1288:⚠️ Potential issue | 🔴 CriticalCopy-paste bug: wrong fallback variables for
$offsetand$cursorLine 1277:
$offset = $values[0] ?? $limit;— should fall back to$offset, not$limit.
Line 1286:$cursor = $values[0] ?? $limit;— should fall back to$cursor, not$limit.If
$values[0]is not set, offset and cursor will be incorrectly set to the limit value instead of preserving their current state.Proposed fix
- $offset = $values[0] ?? $limit; + $offset = $values[0] ?? $offset;- $cursor = $values[0] ?? $limit; + $cursor = $values[0] ?? $cursor;
79-127:⚠️ Potential issue | 🟡 Minor
TYPESarray is missing join and relation-equal constants
isMethod()(lines 454–457) recognizesTYPE_RELATION_EQUAL,TYPE_INNER_JOIN,TYPE_LEFT_JOIN, andTYPE_RIGHT_JOIN, but the publicTYPESarray (lines 79–127) does not include them. While the codebase uses specialized type arrays (JOINS_TYPES,FILTER_TYPES, etc.) for actual validation rather than the generalTYPESarray, the inconsistency creates confusion about the completeness ofTYPESand could introduce bugs if future code relies on it for validation.Add the missing constants to
TYPESfor consistency:Proposed fix
self::TYPE_OR, self::TYPE_ELEM_MATCH, - self::TYPE_REGEX + self::TYPE_REGEX, + self::TYPE_RELATION_EQUAL, + self::TYPE_INNER_JOIN, + self::TYPE_LEFT_JOIN, + self::TYPE_RIGHT_JOIN, ];
🤖 Fix all issues with AI agents
In `@src/Database/Query.php`:
- Line 195: The Query class declares a protected bool $system (set in the
constructor and passed from the select() factory) but never exposes it, making
it dead state; add a public accessor (e.g., isSystem() : bool or getSystem() :
bool) near the other accessors in Query so callers can read the flag, and ensure
its name matches the project's boolean getter convention and is used where
select() supplies the value.
🧹 Nitpick comments (1)
src/Database/Query.php (1)
350-378: Inconsistent exception type — useQueryExceptioninstead of\Exception
getCursorDirection()andgetOrderDirection()throw\Exception, while the rest of the class usesQueryException. This makes it harder for callers to catch query-related errors uniformly.Proposed fix
- throw new \Exception('Invalid method: Get cursor direction on "'.$this->method.'" Query'); + throw new QueryException('Invalid method: Get cursor direction on "'.$this->method.'" Query');- throw new \Exception('Invalid method: Get order direction on "'.$this->method.'" Query'); + throw new QueryException('Invalid method: Get order direction on "'.$this->method.'" Query');
# Conflicts: # src/Database/Adapter/Mongo.php
Summary by CodeRabbit
New Features
Refactor